Skip to content

C++: Support for & and >> in SimpleRangeAnalysis#3277

Merged
dbartol merged 7 commits into
github:masterfrom
geoffw0:rangeshift
Apr 22, 2020
Merged

C++: Support for & and >> in SimpleRangeAnalysis#3277
dbartol merged 7 commits into
github:masterfrom
geoffw0:rangeshift

Conversation

@geoffw0

@geoffw0 geoffw0 commented Apr 16, 2020

Copy link
Copy Markdown
Contributor

Add support for bitwise & and >> (by a constant) in SimpleRangeAnalysis.qll. This will fix #3247.

I've done a bit of performance testing locally, and the query seems to be up to 2% slower in the cases where I can measure a difference.

@geoffw0 geoffw0 added the C++ label Apr 16, 2020
@geoffw0 geoffw0 requested a review from a team as a code owner April 16, 2020 12:16
Comment thread cpp/ql/src/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll Outdated

@dbartol dbartol left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the two more suggested getFullyConverted()s, LGTM.

Comment thread cpp/ql/src/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll Outdated
Comment thread cpp/ql/src/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll Outdated
Co-Authored-By: Dave Bartolomeo <dbartol@github.com>
@geoffw0

geoffw0 commented Apr 22, 2020

Copy link
Copy Markdown
Contributor Author

Thanks @dbartol !

exists(RShiftExpr rsExpr, float left, int right |
rsExpr = expr and
left = getFullyConvertedUpperBounds(rsExpr.getLeftOperand()) and
right = rsExpr.getRightOperand().getValue().toInt() and

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should have .getFullyConverted().

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by #3925.

Comment thread cpp/ql/src/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LGTM.com - false positive C/C++ comparison with wider type

3 participants